Catch agnostic HTTP error types in prefect and swap the pagination raise#24284
Catch agnostic HTTP error types in prefect and swap the pagination raise#24284mwdd146980 wants to merge 1 commit into
Conversation
Widen PrefectClient.http_exceptions to admit the backend-agnostic HTTP error types alongside the requests ones, and raise HTTPInvalidURLError instead of requests' InvalidURL from check_pagination_url so the check stays correct after the httpx migration.
|
Validation Report
Run Passed validations (20)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdb07a1de9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| def __init__(self, url: str, http: RequestsWrapper, log: CheckLoggingAdapter): | ||
| self.http_exceptions = (HTTPError, InvalidURL, ConnectionError, Timeout, JSONDecodeError) | ||
| self.http_exceptions = ( |
There was a problem hiding this comment.
The root AGENTS.md requires a changelog entry for shipped Agent changes, including Python sources under datadog_checks/, but this commit changes prefect/datadog_checks/prefect/check.py without adding a prefect/changelog.d/<PR_NUMBER>.fixed entry. That leaves this fix out of release notes/validation, so add the entry after the PR number is known.
Useful? React with 👍 / 👎.
| HTTPStatusError, | ||
| HTTPInvalidURLError, | ||
| HTTPConnectionError, | ||
| HTTPTimeoutError, |
There was a problem hiding this comment.
When users opt into use_httpx2, HTTPX2Wrapper maps httpx2.LocalProtocolError and generic RequestError to HTTPRequestError (datadog_checks_base/datadog_checks/base/utils/httpx2.py:129), but this tuple only adds the narrower status/URL/connection/timeout classes. In those request-error cases the get/post exceptions bypass the pagination/check handlers and can abort the check instead of logging incomplete data, so include the library-agnostic base request/error type here.
Useful? React with 👍 / 👎.
1 participant
What does this PR do?
Widens
PrefectClient.http_exceptionsto admit the backend-agnostic HTTP error types alongside the existingrequestsones, and swaps theInvalidURLraise incheck_pagination_urlto the agnosticHTTPInvalidURLError. Mirrors the widened tuple into themock_prefect_clienttest fixture.Motivation
Part of the
requeststohttpx2migration. Production funnels every HTTP call through the singleself.http_exceptionstuple, so widening it repairs all eight catch sites at once. Thecheck_pagination_urlraise is the one production behavior change in this PR: it must be swapped together with the tuple widening, since the raised value is caught by that same tuple, decoupling them would produce an unhandled exception rather than a clean test failure.No observable behavior change under the default
requestsbackend: the logged message text is identical either way and the exception never surfaces past the internal catch.Verification
Red-then-green on
test_paginate_events_rejects_external_next_page: swapping only the test assertion first produced a cleanAssertionError(the requestsInvalidURLis not anHTTPInvalidURLError), confirming the right red before the paired production edit landed.ddev test prefectpasses all 5 unit tests,ddev test -fs prefectis clean, and norequestsreference remains intests/test_unit.py.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged